-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes for PowerToys Awake #12215
Fixes for PowerToys Awake #12215
Conversation
This should reduce CPU usage.
@dend Why don't you just follow our Pull Request template? I just want to know what holds you back? Is there anything I'm overlooking here? |
Nothing big @Aaron-Junker - I just wanted to use a very simplified version of it. Since I am just fixing my own tool, and y'all know who I am - thought I would just keep it brief. |
This should be ready for review. Before merging: I'd love a sanity check from folks to clone this PR, build and run PowerToys, and see if Awake works on your machine. I've made a few changes that address the reported bugs, but I want to make sure that it actually works on more than my machine. |
@jaimecbernardo is the module enabled in PowerToys settings? Can you check and see if the process is running in Task Manager ( |
@dend
|
@dend |
@mykhailopylyp - I will double-check once again to see what is going on with the icon, and where I am failing with |
@mykhailopylyp can you please confirm that you are using the content from this PR? |
This change introduces support for: - Proper event handling across the two apps (Awake and runner). - Removal of unnecessary functions.
Sorry for the confusion. This crash wasn't caused by this branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, tested, works as expected. Thank you for your contribution!
Awesome - thanks for checking @mykhailopylyp 👏 @crutkas anything else we need here before we can merge? |
@dedavis6797 FYI - let me know if I need to make any other changes for us to merge this PR. |
Let's wait for @jaimecbernardo's review as he found some issues |
I'll do a review today still. And if it's all OK, I'll merge it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After keeping the computer awake temporarily, the computer can go to sleep after the time we set, but the setting never changes in the UI. The visuals show as if the mode is still "Keep awake temporarily" instead of resetting to "Off (Passive)".
This is still occurring to me. After being kept awake temporarily, the behavior returns as if it is "Off (Passive)" but the UI still shows as if the setting is to be kept awake temporarily.
@jaimecbernardo this is by-design - the UI in the Settings view doesn't automatically update today. This will be an area I will look at improving in a separate release. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes LGTM.
Great work!
Addresses the following issues: